-
-
Notifications
You must be signed in to change notification settings - Fork 343
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable flake8 annotations #3098
Enable flake8 annotations #3098
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3098 +/- ##
=======================================
Coverage 99.63% 99.63%
=======================================
Files 122 122
Lines 18380 18380
Branches 1222 1222
=======================================
Hits 18312 18312
Misses 47 47
Partials 21 21
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, the self/cls issue is because this is enabling the deprecated ANN101
/ANN102
rules. Those should be disabled. Also, I really don't see the point anyway in running any of our linters on notes-to-self
. Those are just historical test/example scripts, they're not supposed to be robust or anything. It just produces churn...
0f78683
to
c05eaa6
Compare
Did a force push basically redoing everything, much easier than manually undoing adding self everywhere |
def create_pipe_from_child_output(): | ||
def create_pipe_from_child_output() -> tuple[PipeReceiveStream, int]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a mypy setting for catching this as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are referring to disallow_incomplete_defs
and check_untyped_defs
, those two are already enabled for Trio.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant disallow_untyped_defs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
disallow_untyped_defs
is also already enabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then why did this not get caught by mypy previously??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really sure
Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>
tbh, is there any reason to keep them around at all anymore? We could dump them in a gist and save a link somewhere if we want to save them somewhere (other than in the git history) |
Could create a git tag to mark which commit is is with the last version. |
Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>
# Don't check annotations in notes-to-self | ||
'notes-to-self/*.py' = ['ANN001', 'ANN002', 'ANN003', 'ANN201', 'ANN202', 'ANN204'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whichever is merged latest of this and #3117 can remove these lines.
agreed. disable ANN401 for now and enable `disallow_any_explicit` in a separate PR. Possibly same for `disallow_untyped_defs`. _Originally posted by @jakkdl in python-trio#3098 (comment)
Planning on merging this in a day or two unless anyone has any other comments. |
In this pull request, we enable ruff's
flake8-annotations
rule.One of the more major things this rule complains about is missing type annotations on
self
andcls
arguments, so there are a lot of places that gotSelf
andtype[Self]
annotations added respectfully, as there is not a way to specifically ignoreself
orcls
in particular without missing others.